Conversation
fix: update type matching logic in ContentMapper for improved conditi…
…ema merging and UID handling; update ContentMapper to support nested modular blocks and prevent duplicate selections fix - CMG-812, CMG-818, CMG-823, CMG-824, CMG-831
…jsdom to 23.2.0, multer to 2.1.1, and rollup to 4.59.0; add overrides for various packages to ensure compatibility Snyk Issue Resolved
…or rollup and overrides in upload-api
…oved type safety and clarity
Bugfix/cmg 823
There was a problem hiding this comment.
Pull request overview
This PR updates dependency versions/overrides for the Upload API and UI packages, and enhances migration mapping logic (UI ContentMapper + API utilities) to better handle nested schemas (groups/modular blocks) and AEM field extraction.
Changes:
- Bump
jsdomandmulterinupload-api, and introduce additionaloverridesto address transitive dependency versions. - Update UI dependency overrides/lockfile, and extend ContentMapper to support nested modular blocks and improved auto-mapping/clearing behavior.
- Refactor content type merge logic and adjust AEM group-field extraction to use source path keys more reliably.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| upload-api/package.json | Updates key dependencies and adds overrides for transitive packages. |
| upload-api/package-lock.json | Lockfile updates reflecting dependency bumps and transitive changes. |
| ui/src/components/ContentMapper/index.tsx | Adds nested modular block handling and new “cleared fields” tracking to control auto-mapping. |
| ui/src/components/ContentMapper/contentMapper.interface.ts | Extends OptionsType with uid to support mapping selection. |
| ui/package.json | Adds dependency overrides to enforce patched versions. |
| ui/package-lock.json | Lockfile updates reflecting overrides and dependency bumps. |
| api/src/utils/content-type-creator.utils.ts | Refactors CT merge to recursively merge group/blocks schemas. |
| api/src/services/aem.service.ts | Improves group-field extraction by using AEM source-key resolution and updated multiple logic. |
Files not reviewed (2)
- ui/package-lock.json: Language not supported
- upload-api/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| "tmp": "^0.2.4", | ||
| "ajv": "^8.18.0", | ||
| "diff": "^4.0.4", | ||
| "fast-xml-parser": "^5.3.8", | ||
| "minimatch": "^9.0.7", | ||
| "qs": "^6.14.2" |
There was a problem hiding this comment.
The overrides for diff and minimatch use caret ranges (^4.0.4, ^9.0.7) but the updated lockfile resolves to newer major versions (e.g. diff@8.x, minimatch@10.x). With npm overrides, these ranges can force a different (older) major at install time and make package-lock.json drift or installs non-reproducible. Consider changing these to >= minimums (or to the exact major actually locked) and regenerating the lockfile so overrides + lock agree. Also, if the intent for tmp is “at least 0.2.4”, >=0.2.4 is safer than ^0.2.4.
| "tmp": "^0.2.4", | |
| "ajv": "^8.18.0", | |
| "diff": "^4.0.4", | |
| "fast-xml-parser": "^5.3.8", | |
| "minimatch": "^9.0.7", | |
| "qs": "^6.14.2" | |
| "tmp": ">=0.2.4", | |
| "ajv": ">=8.18.0", | |
| "diff": ">=4.0.4", | |
| "fast-xml-parser": ">=5.3.8", | |
| "minimatch": ">=9.0.7", | |
| "qs": ">=6.14.2" |
| setExistingField((prev) => { | ||
| const next = { ...prev }; | ||
| allDescendants.forEach((item) => delete next[item?.backupFieldUid]); | ||
| allDescendants?.forEach((item) => delete next[item?.backupFieldUid]); | ||
| next[backupFieldUid] = { label: selectedValue?.label, value: selectedValue?.value }; | ||
| return next; |
There was a problem hiding this comment.
When selectedValue is cleared, this branch still leaves an existingField[backupFieldUid] entry with label/value undefined. Downstream logic in this component treats existingField[*].label as a string (e.g., for disabling options / building selectedOptions), so clearing should remove the key entirely (and avoid propagating an undefined label) rather than storing an undefined mapping.
|
|
||
| const OptionValue: FieldTypes = | ||
| OptionsForRow?.length === 1 && (existingField[data?.backupFieldUid] || updatedExstingField[data?.backupFieldUid]) && | ||
| (OptionsForRow?.[0]?.value?.uid === 'url' || OptionsForRow?.[0]?.value?.uid === 'title' || OptionsForRow?.[0]?.value?.data_type === 'group' || OptionsForRow?.[0]?.value?.data_type === 'reference' | ||
| (OptionsForRow?.[0]?.value?.uid === 'url' || OptionsForRow?.[0]?.value?.uid === 'title' || (OptionsForRow?.[0]?.value?.data_type === 'group' && !data?.uid?.includes('.')) || OptionsForRow?.[0]?.value?.data_type === 'reference' | ||
|
|
||
| ) | ||
| ? { |
There was a problem hiding this comment.
OptionValue builds the auto-mapped Select value using OptionsForRow[0].value.display_name, but the auto-mapping logic above now writes contentstackField using OptionsForRow[0].label (which can include the full path like A > B). This mismatch can cause the Select to display a different label than what gets persisted/added to selectedOptions, and can break the disable/dedup logic. Use the same source (option.label / option.uid) consistently for both display and persisted mapping.
No description provided.